[BUG]: Fix TransformedTargetRegressor update() transformer usage and transformer=None predict handling#1028
Conversation
…gitignore and wheelhouse artifacts
fkiraly
left a comment
There was a problem hiding this comment.
Thanks for the fix!
I think the reason that this did not get caught by the tests is that the get_test_params did have no example where the regressor was capable of update, I think.
Could you add such an example?
|
@fkiraly, youu are exactly right on the likely root cause. |
|
will update |
|
so, i will be doing it by adding the 3rd params in the |
|
Hi, @fkiraly the changes you asked are done |
|
thanks - can you kindly name the test that would fail before additions in this PR? And which now passes? |
|
Hi @fkiraly ,There are two tests that failed before the fix in the main branch
now on the current branch direkkakkar319-ops: (venv) direk@dkakkar:/mnt/f/skpro$ git branch --show-current
TransformedTargetRegressor-DirekKakkar
(venv) direk@dkakkar:/mnt/f/skpro$ python -m pytest skpro/regression/tests/test_ttr.py -v -p no:randomly --no-header -rN --override-ini="addopts="
============================================================================= test session starts =============================================================================
collected 2 items
skpro/regression/tests/test_ttr.py::test_ttr_without_transformer_predicts_and_returns_distribution PASSED [ 50%]
skpro/regression/tests/test_ttr.py::test_ttr_update_applies_transformer_before_delegating PASSED [100%]
============================================================================== 2 passed in 5.10s ==============================================================================
(venv) direk@dkakkar:/mnt/f/skpro$ before they failed (venv) direk@dkakkar:/mnt/f/skpro$ git show main:skpro/regression/compose/_ttr.py > skpro/regression/compose/_ttr.py && python -m pytest skpro/regression/tests/test_ttr.py -
v -p no:randomly --no-header -rN --override-ini="addopts="
============================================================================= test session starts =============================================================================
collected 2 items
skpro/regression/tests/test_ttr.py::test_ttr_without_transformer_predicts_and_returns_distribution FAILED [ 50%]
skpro/regression/tests/test_ttr.py::test_ttr_update_applies_transformer_before_delegating FAILED [100%]
...........................
============================================================================== 2 failed in 8.07s ==============================================================================
(venv) direk@dkakkar:/mnt/f/skpro$ |



What does this implement/fix?
This PR fixes a few issues in
TransformedTargetRegressor(skpro/regression/compose/_ttr.py) where valid usage could fail at runtime._updatenow correctly callsself.transformer_.transform(...)instead of invoking the transformer directly.transformer=None(the default), which now works consistently across:1.
_predict_predict_quantiles_predict_interval_predict_probaDataFrameindex and column structure when transformingyinside_update.I also added targeted regression tests in
skpro/regression/tests/test_ttr.pyto cover:transformer=Noneforpredictandpredict_probaupdateDoes this introduce a new dependency?
No, no new dependencies are introduced.
What should reviewers focus on?
Feedback would be especially helpful on:
transformer=Nonehandling across prediction methods_updatebehavior with sklearn-style transformers (transform,inverse_transform)ypath is appropriateTests
Yes, tests have been added:
test_ttr_without_transformer_predicts_and_returns_distributiontest_ttr_update_applies_transformer_before_delegatingAdditional notes
This is a bugfix-only PR. No API changes and no new dependencies.
PR checklist
For all contributions
For new estimators
Local Test
Validation (local CI-equivalent checks)
pre-commit run --files skpro/regression/compose/_ttr.py skpro/regression/tests/test_ttr.pypassed.13406 passed, 33 skipped, 468 warningsEvidence